Delta Lake Catalog Exporter #7078

Jonathan-Rosenberg · 2023-11-29T15:19:27Z

Closes #6965

Changes

Change the encoding/json lua library to accept an optional options table. This is in order to pass an indentation if needed.
Implement a delta client for lua which can get a delta lake table.
Pass listeningAddress variable to the actions service. This is to allow hooks to communicate back to the running lakeFS server if they run on the same host, e.g. lua hooks.
Implement the delta_exporter lua function that allows configuring a delta table exporting hook.
Extend the lakeFS lua client with stat_object and get_repo operations.

NOTICE 1

This PR depends on the merging of treeverse/delta-go#2.

NOTICE 2

Documentation will be added in a different PR.

How was this tested?

Manually tested on the following Delta Lake log constructs:

A delta log with no checkpoints (i.e. starts with entry 000000000000000000.json).
A delta log with a checkpoint and past logs (e.g. 000000000000000004.json and all other json entries up to and including checkpoint 00000000000000000010.checkpoint.parquet, and entry 00000000000000000010.json)
A delta log with a checkpoint and no past logs (e.g. starting from 00000000000000000010.checkpoint.parquet, followed by a number of log entries).

change response of delta_exporter, remove prints, remove old impl

examples/hooks/delta_lake_S3_export.lua

pkg/actions/lua/formats/delta.go

Isan-Rivkin · 2023-11-29T17:39:07Z

pkg/actions/lua/lakefs/catalogexport/delta_exporter.lua

+    if ns == nil then
+        error("failed getting storage namespace for repo " .. repo)
+    end
+    local delta = formats.delta_client("s3", lakefs_key, lakefs_secret, region)


Not fully in the context yet but I can't help not wonder already.
Is there a reason to depend on passing lakeFS credentials here?

Until now we worked with the policy that lua lakeFS client is using the context of the logged in user in the context during runtime to authenticate with lakeFS api.

As mentioned F2F, this is due to the need to supply the delta-go package's client AWS key and secret to communicate with the S3 gateway.
On top of that, the administrator might configure a user that has permission to read the Delta table from its location such that every commit will export it even if the user that committed it doesn't have the right permissions.

100% 👍
Should document (in general I know not in this PR, what lakeFS permissions are required)
The technical problem reusing same technique as lakeFS lua client is because of the creds provider that expects credentials. 😢

CredsProvider: aws.CredentialsProviderFunc(func(ctx context.Context) (aws.Credentials, error) {

pkg/actions/kv_run_results_iterator_test.go

pkg/actions/service.go

pkg/actions/lua/stdlib.go

Isan-Rivkin · 2023-11-30T06:48:31Z

pkg/actions/lua/lakefs/catalogexport/internal.lua

+local function sortedKeys(query, sortFunction)
+    local keys, len = {}, 0
+    for k,_ in pairs(query) do
+        len = len + 1
+        keys[len] = k
+    end
+    if sortFunction ~= nil then
+        table.sort(keys, sortFunction)
+    else
+        table.sort(keys)
+    end
+
+    return keys
+end


Why is the sort function needed here, don't see its being used? If it's not needed let's remove it.
If you prefer to keep it then no need to check for nil:

Suggested change

local function sortedKeys(query, sortFunction)

local keys, len = {}, 0

for k,_ in pairs(query) do

len = len + 1

keys[len] = k

end

if sortFunction ~= nil then

table.sort(keys, sortFunction)

else

table.sort(keys)

end

return keys

end

local function sortedKeys(query, sortFunction)

local sort = sortFunction or

local keys, len = {}, 0

for k,_ in pairs(query) do

len = len + 1

keys[len] = k

end

table.sort(keys, sortFunction)

return keys

end

It's used in delta_exporter.
Not sure I understand the suggested change, but I do agree that the nil check is redundant. 🙏

I meant specifically the sortFunction argument in the function, I don't see it being used anywhere?
If you prefer to leave it regardless, please add a comment on the sortFunction expected signature otherwise it'll be hard to use it without going into the implementation of sortedKeys

sortFunction is passed to table.sort to apply the sorting logic (otherwise it uses default lexicographic sort). I don't need to pass, but since it's a util function, the API should suggest a way to sort keys.
I'll add a comment.

pkg/actions/lua/lakefs/catalogexport/delta_exporter.lua

pkg/actions/lua/lakefs/catalogexport/storage_utils.lua

Lua refactoring Remove unneeded `listeningAddress` member from `DeltaClient`

lua-require packages

Isan-Rivkin

Review WIP - if you prefer all at once LMK 🙏

pkg/actions/lua/lakefs/catalogexport/internal.lua

pkg/actions/lua/lakefs/catalogexport/delta_exporter.lua

pkg/actions/lua/lakefs/client.go

Isan-Rivkin

General review comment - what is the plan about tests?

Reference to glue and symlink exporter test integration in esti - can we add a test here?
Lua package unit testing - see reference hive_partition_pager.lua test

pkg/actions/lua/lakefs/catalogexport/delta_exporter.lua

pkg/actions/lua/formats/delta.go

# Conflicts: # go.mod # go.sum

github-actions · 2023-12-03T12:38:19Z

♻️ PR Preview 2b9f8a2 has been successfully destroyed since this PR has been closed.

_{🤖 By surge-preview}

…ient since the storage namespace can be fetched from the global action object

…ete key name (including ".json" suffix)

Jonathan-Rosenberg

@Isan-Rivkin, tests will be added on a different PR.
Thank you 🙏

pkg/actions/lua/formats/delta.go

…to the export_delta_log function

pkg/actions/lua/stdlib.go

pkg/actions/lua/lakefs/catalogexport/delta_exporter.lua

pkg/actions/lua/formats/delta.go

…sing the S3 gateway). Use regex to validate lakeFS address

Isan-Rivkin

LGTM!
I left 2 comments un-resolved as reminders incase it's missed - feel free resolve.
Thank you for taking the time and addressing the comments!
Can't wait to export to Delta 🕺

Jonathan-Rosenberg added 12 commits November 23, 2023 15:50

delta client impl

c446901

delta exporter

8e8ddca

Use the earliest available version to build the Delta log

8bb3f1c

pass listen address to hooks

5adf15d

add stat_object api call to the lakefs lua client

37adb94

add get_repo to eventually fetch its storage namespace

cd7b8b6

delta lake exporter example

c3bc982

add error return. remove redundant lines. add godocs for some funcitons

8a81e4e

extract get_storage_namespace to the internal lua file.

c4f520a

change response of delta_exporter, remove prints, remove old impl

remove comment

b2b81aa

go mod tidy

587ed18

pass aws properties directly to fetchS3Table

bde6581

Jonathan-Rosenberg added the include-changelog PR description should be included in next release changelog label Nov 29, 2023

Jonathan-Rosenberg requested review from nopcoder, ozkatz, itaiad200 and Isan-Rivkin and removed request for ozkatz November 29, 2023 15:19

json encoding refactor

4df6b5a

Jonathan-Rosenberg requested review from johnnyaug and removed request for nopcoder November 29, 2023 16:34

Jonathan-Rosenberg added 4 commits November 29, 2023 19:00

fix load test

c49c42e

fix tests

e5013b9

fix tests

7318c64

goimports

2bac82e

Isan-Rivkin requested changes Nov 29, 2023

View reviewed changes

Isan-Rivkin requested changes Nov 30, 2023

View reviewed changes

Jonathan-Rosenberg added 3 commits November 30, 2023 11:55

add indentation to lua test

9bd7a76

remove storage_utils.lua

855a443

Rename ListeningAddress (and listeningAddress) to ServerAddress.

74eb76b

Lua refactoring Remove unneeded `listeningAddress` member from `DeltaClient`

Jonathan-Rosenberg added 2 commits November 30, 2023 12:51

fix test

0866a3c

lint

d0bb96e

Jonathan-Rosenberg requested a review from Isan-Rivkin November 30, 2023 12:00

Jonathan-Rosenberg added 2 commits November 30, 2023 14:58

pass a delta client to the export_delta function

e5ca6b8

lua-require packages

upgrade aws-sdk-v2

0b6853d

Isan-Rivkin requested changes Dec 3, 2023

View reviewed changes

Jonathan-Rosenberg added 3 commits December 3, 2023 14:30

Merge branch 'master' into feature/catalog-export/delta

a028ac8

# Conflicts: # go.mod # go.sum

go mod tidy

e32dc16

use get_storage_prefix from the internal package. Remove it from docs

aa476a8

Jonathan-Rosenberg added 4 commits December 3, 2023 14:39

remove get_storage_namespace and the get_repo method from lakeFS cl…

0bae4ba

…ient since the storage namespace can be fetched from the global action object

use get_storage_prefix from the internal package

a603196

use correct error when returning

9e522f5

comment the delta_log_entry_key_generator function and return a compl…

178107d

…ete key name (including ".json" suffix)

Jonathan-Rosenberg commented Dec 3, 2023

View reviewed changes

pkg/actions/lua/formats/delta.go Show resolved Hide resolved

Jonathan-Rosenberg requested a review from Isan-Rivkin December 3, 2023 13:07

Jonathan-Rosenberg added 3 commits December 3, 2023 15:16

use a writer_client function instead of a storage client when passed …

a687c91

…to the export_delta_log function

updated version of delta go

98c3ac2

severAddress -> lakeFSAddr + comment

590f2e3

Isan-Rivkin reviewed Dec 3, 2023

View reviewed changes

pkg/actions/lua/stdlib.go Outdated Show resolved Hide resolved

pkg/actions/lua/lakefs/catalogexport/delta_exporter.lua Outdated Show resolved Hide resolved

Isan-Rivkin requested changes Dec 3, 2023

View reviewed changes

pkg/actions/lua/formats/delta.go Outdated Show resolved Hide resolved

Jonathan-Rosenberg added 2 commits December 6, 2023 11:56

Merge branch 'master' into feature/catalog-export/delta

9de27d8

change delta client builder API and drop storage type (since always u…

d9cde83

…sing the S3 gateway). Use regex to validate lakeFS address

Jonathan-Rosenberg requested a review from Isan-Rivkin December 6, 2023 10:34

Jonathan-Rosenberg added 2 commits December 6, 2023 12:37

rename the lakeFS address field and add clarifying comment

dd4c81d

Merge branch 'master' into feature/catalog-export/delta

2b9f8a2

Isan-Rivkin approved these changes Dec 6, 2023

View reviewed changes

Jonathan-Rosenberg merged commit bb38ba0 into master Dec 11, 2023
35 checks passed

Jonathan-Rosenberg deleted the feature/catalog-export/delta branch December 11, 2023 09:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delta Lake Catalog Exporter #7078

Delta Lake Catalog Exporter #7078

Jonathan-Rosenberg commented Nov 29, 2023

Isan-Rivkin Nov 29, 2023

Jonathan-Rosenberg Nov 30, 2023

Isan-Rivkin Dec 3, 2023 •

edited

Loading

Isan-Rivkin Nov 30, 2023

Jonathan-Rosenberg Nov 30, 2023

Isan-Rivkin Dec 3, 2023

Jonathan-Rosenberg Dec 3, 2023 •

edited

Loading

Isan-Rivkin left a comment

Isan-Rivkin left a comment

github-actions bot commented Dec 3, 2023 •

edited

Loading

Jonathan-Rosenberg left a comment

Isan-Rivkin left a comment

Delta Lake Catalog Exporter #7078

Delta Lake Catalog Exporter #7078

Conversation

Jonathan-Rosenberg commented Nov 29, 2023

Changes

NOTICE 1

NOTICE 2

How was this tested?

Isan-Rivkin Nov 29, 2023

Choose a reason for hiding this comment

Jonathan-Rosenberg Nov 30, 2023

Choose a reason for hiding this comment

Isan-Rivkin Dec 3, 2023 • edited Loading

Choose a reason for hiding this comment

Isan-Rivkin Nov 30, 2023

Choose a reason for hiding this comment

Jonathan-Rosenberg Nov 30, 2023

Choose a reason for hiding this comment

Isan-Rivkin Dec 3, 2023

Choose a reason for hiding this comment

Jonathan-Rosenberg Dec 3, 2023 • edited Loading

Choose a reason for hiding this comment

Isan-Rivkin left a comment

Choose a reason for hiding this comment

Isan-Rivkin left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 3, 2023 • edited Loading

Jonathan-Rosenberg left a comment

Choose a reason for hiding this comment

Isan-Rivkin left a comment

Choose a reason for hiding this comment

Isan-Rivkin Dec 3, 2023 •

edited

Loading

Jonathan-Rosenberg Dec 3, 2023 •

edited

Loading

github-actions bot commented Dec 3, 2023 •

edited

Loading